Skip to content

Exclude Redux lib from builds #178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

kaidjohnson
Copy link

I agree that the redux library should not be a baked-in dependency of the /dist builds of ng-redux. Simple enough to exclude using rollup globals.

Fixes #70

@deini
Copy link
Collaborator

deini commented Dec 19, 2017

Hey @kaidjohnson thanks for opening this PR! A couple of things:

  1. This is going to be a Breaking change, so please open this PR against the release/4.0.0 branch instead of master 🙏

  2. Since 4.0.0 wont have a /dist directory lets remove those changes for now

  3. Lets move redux from dependencies to devDependencies in package.json and then add redux as a peer dependency too:

  "peerDependencies": {
    "redux": "^3.0.0"
  },
  1. Instead of having it as globals let make it part of externals you can checkout rollups guide on peer Dependencies.

  2. You will need to add the externals to the umd build, however since we moved the dependency to peerDependencies it will break es/cjs builds, so you would have to modify that external to accept what it currently does + redux.

  3. We are going to start enforcing conventional commits (chore: setup standard-version #180, chore: lint commit messages #181) so please change your commit to follow the standard.
    Eg: something like:

chore(build): Make redux a peerDependency

BREAKING CHANGE: Redux is not going to be part of the umd bundle anymore

Let me know if you have any questions or need help with any of the points stated above.

@kaidjohnson kaidjohnson changed the base branch from master to release/4.0.0 January 2, 2018 21:10
@kaidjohnson kaidjohnson changed the base branch from release/4.0.0 to master January 2, 2018 21:21
@kaidjohnson
Copy link
Author

Looks like release/4.0.0 already includes redux as an external, so the issue should be resolved already on that branch. Fantastic! And thanks for the quick response :)

@kaidjohnson kaidjohnson closed this Jan 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants